Fix the issue of unused assignment from MIR liveness checking #149072
Conversation
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
rustbot
added
S-waiting-on-review
| // If this place was dropped and has non-trivial drop, | ||
| // skip reporting field assignments. | ||
| if !is_direct && is_maybe_drop_guard { | ||
| continue; |
There was a problem hiding this comment.
this is for avoiding report unused_assign for this line of code:
https://github.com/rust-lang/rust/blob/main/library/std/src/io/mod.rs#L393
There was a problem hiding this comment.
Why would there be an unused_assign for that line irrespective of whether there is a Drop impl?
There was a problem hiding this comment.
If there's no subsequent code that explicitly reads g.len, this assignment appears "unused".
I think the liveness checking in MIR only knows that the entire g will be dropped, but doesn't know the drop process will read g.len. The is_direct here is used to distinguish this case and avoid false positives for field assignments that Drop may use.
There was a problem hiding this comment.
In the past fields weren't checked, only whole variables, right? Why was that changed?
There was a problem hiding this comment.
The indirect statement is added here:
it seems to be trying to record the access for a variable with a field:
let mut waiter_queue = .... ;
...
waiter_queue.set_state_on_drop_to = ..@cjgillot correct me if I'm wrong.
There was a problem hiding this comment.
another issue which related to Drop, but this PR does not fix it:
maybe we need a more general fix.
| IndexEntry::Occupied(mut o) => { | ||
| // There were already a sighting. Mark this statement as live if it | ||
| // was, to avoid false positives. | ||
| o.get_mut().live |= live.contains(index); |
There was a problem hiding this comment.
Should we update direct too?
There was a problem hiding this comment.
yeah, I fixed it and added a test case.
chenyukang
force-pushed
the
yukang-fix-unused-148960
branch
from
57c1dcf to
4930d3e
Compare
chenyukang
force-pushed
the
yukang-fix-unused-148960
branch
from
b7c9e8d to
00f3155
Compare
5 participants
Fixes #148960
Fixes #148418
r? @davidtwco
cc @cjgillot
My first try on MIR related code, so it may not be the best fix.